Add a option for filtering vars to the ns-vars middleware #605
Conversation
liquidz
force-pushed
the
feature/ns-vars
branch
from
3608c07 to
0a5a634
Compare
src/cider/nrepl/middleware/ns.clj
Outdated
| (u/update-keys name) | ||
| (into (sorted-map)))) | ||
| (defn ns-vars-clj [ns & [include-privates?]] | ||
| (let [fetch-vars (if include-privates? |
There was a problem hiding this comment.
We can also use var-query here directly. That would simplify a bit the code.
There was a problem hiding this comment.
Thank you for reviewing!
I missed var-query API. I'll fix.
There was a problem hiding this comment.
@bbatsov Fixed!
If we have orchard.query/vars for clojurescript, I think it seems better to use var-query API like test-var-query op.
There was a problem hiding this comment.
We don't have it yet (something that @arichiardi can probably tackle), but I think we can still adopt the same API and just "fake" it on the cljs side for now (by ignoring everything but the private filter).
There was a problem hiding this comment.
@bbatsov OK! I fixed to take var-query option for filtering vars.
On the cljs side, it only support private? query currently.
|
The PR looks good overall. I mostly wonder if we should try to use here the var-query API, as this would give more flexibility to the ns filtering in general. |
liquidz
force-pushed
the
feature/ns-vars
branch
from
0a5a634 to
e1cb0af
Compare
Codecov Report
@@ Coverage Diff @@
## master #605 +/- ##
========================================
Coverage ? 76.5%
========================================
Files ? 37
Lines ? 2409
Branches ? 128
========================================
Hits ? 1843
Misses ? 438
Partials ? 128
Continue to review full report at Codecov.
|
liquidz
force-pushed
the
feature/ns-vars
branch
from
e1cb0af to
9964b4c
Compare
liquidz
changed the title
liquidz
force-pushed
the
feature/ns-vars
branch
from
9964b4c to
0431a95
Compare
|
Fantastic work! Thanks! 🙇 |
|
A newbie question on this, what is the difference between this The kind of seem redundant but maybe |
|
I was referring to https://github.com/clojure-emacs/orchard/blob/master/src/orchard/query.clj How is |
|
Oh ok it is a different use case...I was just wondering if |
Fix #566.